Skip to content

Conversation

@krisukox
Copy link
Contributor

@krisukox krisukox commented Jun 13, 2023

This PR adds ability to use x86_64 sysroot with llvm toolchain. Usage:
bazel build //... --@rules_swiftnav//cc:enable_sysroot=true

Related PR
swift-nav/swift-toolchains#19

@krisukox krisukox marked this pull request as ready for review June 13, 2023 14:01
@krisukox krisukox requested a review from jungleraptor June 13, 2023 14:09
Copy link
Contributor

@jungleraptor jungleraptor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would also like a link to a test PR I can run so we can test these changes on mac.

@krisukox
Copy link
Contributor Author

@isaactorz Here is the test PR: https://github.com/swift-nav/orion/pull/5903

fail(target_system_name + " is not a target tripplet")

cross_compile = host_system_name != target_system_name
use_bundled_libcpp = builtin_sysroot != None and not is_darwin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
use_bundled_libcpp = builtin_sysroot != None and not is_darwin
use_libstdcpp = builtin_sysroot != None and not is_darwin

nit - the variable name doesn't match the logic. When this is true we should use libstdc++ not libc++

Copy link
Contributor

@jungleraptor jungleraptor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@krisukox krisukox merged commit 2a6ac7f into main Jun 16, 2023
@krisukox krisukox deleted the krisukox/x86_sysroot branch June 16, 2023 09:45
jungleraptor added a commit that referenced this pull request Jun 22, 2023
Fixes #92.

We made it so that the condition that chooses between libstdc++ and libc++
always ends up selecting libstdc++. See PR for more details.

The fix required a fairly non-trivial refactor of our toolchain configuration
algorithm in cc_toolchain_config to support all the configurations we want.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants